Add a wizard component for first time configurations. #286
Conversation
StevenDufresne
changed the title
|
Using the tab key I can still navigate around behind the wizard. Probably need to trap the tab navigation within. |
|
Generally we use sentence case for titles, and I can see a lot of title case. The bit that sticks out for me is the title below the logo. 
Overall I think this could do with some design input; as well as the copy the progress bar details could be refined a little (line thickness, icons). |
|
These back arrows are still using the old blue
|
I've lowercased that option titles. The main titles are existing and enforced using CSS. I think we can address that later if needed. |
I've updated this here: 06f3e14 |
| const { navigateToScreen, screen } = useContext( GlobalContext ); | ||
| const steps = [ | ||
| { | ||
| title: 'Choose an authentication method', |
There was a problem hiding this comment.
Do these strings need to be translatable?
There was a problem hiding this comment.
Not available for this project yet.
It's logged: #273
| <div className=""> | ||
| <h3>Two-factor authentication setup is now complete! 🎉</h3> | ||
| <p> | ||
| To ensure the highest level of security for your account, please remember to | ||
| keep your authentication methods up-to-date. We recommend configuring | ||
| multiple authentication methods to guarantee you always have access to your | ||
| account. | ||
| </p> | ||
| <div className="wporg-2fa__submit-actions"> | ||
| <Button | ||
| onClick={ () => { | ||
| const redirectTo = new URLSearchParams( | ||
| window.location.search | ||
| ).get( 'redirect_to' ); | ||
|
|
||
| if ( redirectTo && isValidUrl( redirectTo ) ) { | ||
| window.location.href = redirectTo; | ||
| } else { | ||
| window.location.href = | ||
| '//profiles.wordpress.org/me/profile/edit/group/1'; | ||
| } | ||
| } } | ||
| isPrimary | ||
| > | ||
| Continue | ||
| </Button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Any reason not to extract this as a component too?
| }; | ||
| }, [ screen ] ); | ||
|
|
||
| const currentStepIndex = screens[ screen ].stepIndex; |
There was a problem hiding this comment.
We could do with a check here for the screen existing. Between switching from prod to sandbox I ended up on ?first-time=true&screen=account-status, resulting in an error. Shouldn't be possible normally of course but it highlighted an edge case.
| }; | ||
|
|
||
| return ( | ||
| <div className="wporg-2fa__progress-bar"> |
There was a problem hiding this comment.
We could likely make this element accessible without much effort.
|
Backup codes print view could do with some elements being hidden 
|
|
This might be an existing issue but the back button and title don't fit 
|
|
And the security key screen is collapsing 
|
|
The outline focus style on these radio elements should be removed imo. The focus ring around the whole option and color change on the input should be enough. 
|
|
Nice, looks good! Appreciate you using the new typography, colors, and componentry to build this out. Overall, I wouldn't block this on any one particular piece, but a few things could be refined a bit. In no particular order:
We should establish and adhere to a UI text-case guide. I think one exists, but the link to it escapes me. If I'm not wrong, it's sentence case with capitals for only proper nouns. In this case, it would be "Choose an authentication method", and probably "Configure two-factor", unless "two-factor" counts as a proper noun?
Are the icons scaled down? With few exceptions we should only use those icons in 24x24px. In this case you might also want to replace the icons with numbers, 1, 2, 3 instead, as the icons themselves are not perfect. Can you apply a 1.5px stroke-width both for the horizontal bar, and for the inner stroke of the circle?
We can refine the Back button a bit: no underline, 24x24 small-chevron icon, same blueberry color. This is similar to what we did for older 2fa mockups. 
Do we have yet a new notice component based on these designs from @fcoveram?
If not, I would simply make the text bold and not box it up in that yelllow notice piece. It's an old icon, and old colors, and there's a checkbox below to make sure you understand.
Can these buttons be right below the codes themseles?
Instead of "All Finished", should it be "Finish" or "Complete", or even "I understand"? Can it be a disabled solid button instead of a disabled outline button? I.e. this:
Can we omit "Setup Complete" here? Since there's no background, the centered heading becomes a bit off-balanced given there's also a heading right below. I'd also tend to omit the emoji here. If we want something celebratory, we could potentially revisit the animation we explored for the past mockups: 
|
This plugin was built using I wonder if we can remove it in favor of hiding the "back" button when codes are generated that need saving and updating the copy to include some of this language? |
It can, but if we do, lets do that for all the other buttons as well. Can we open a new ticket for that? |
It's probably easier to remove in the header in the content than re-working the component title. Its probably not as important. |
This reverts commit 0b63573.
Co-authored-by: Adam Wood <[email protected]>
StevenDufresne
force-pushed
the
try/setup-wizard
branch
from
2a1406a to
142b57d
Compare
Just to understand, the button component by default has an outline-look when disabled, regardless of its base variant? If yes, I'll open an issue, I'd expect primary buttons to remain primary-looking when disabled. |
We have a few use cases in different components where the button defaults to disabled, we'll want to keep those consistent whether the button is outlined or filled in. |
|
Exactly, if it's filled, it should always be filled (and have a disabled state), etc. It sounds like that's the case here, so why's it outlined if it's primary when not-disabled? Forgive me if I still miss nuance. Past experience tells me the best disabled state is just an opacity change (e.g. |
|
Closing, we'll go with an inline version: #297 |
It's likely that we enforce 2fa for plugins/theme committers on wp.org. This PR adds a wizard that we can show to users on login that will immediately get them into the configuration flow.
Main changes
script.jscode intosettings.jsso we can choose different UIs.onSuccessevents to components to be used in the wizard.Screenshots
How to test
Without local modifications to the underlying WP 2fa plugins, you can't test this. This is best tested in a sandbox.
Note, if you use an existing account, the wizard will show the "configured states" which isn't expected for regular users.